Skip to content

[4기 - 박형진] SpringBoot Part3 Weekly Mission 두 번째 PR 제출합니다. #851

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: legowww
Choose a base branch
from

Conversation

legowww
Copy link

@legowww legowww commented Jul 17, 2023

📌 과제 설명

  • Spring MVC를 적용하여 JSON과 XML을 지원하는 REST API 개발하기

👩‍💻 요구 사항과 구현 내용

JSON, XML 컨트롤러를 구현하고
CORS 설정을 추가했습니다.

✅ PR 포인트 & 궁금한 점

Comment on lines 46 to 54
@GetMapping("/date")
public Response<List<VoucherDto>> betweenDatesCreatedVouchers(
@RequestParam("startDate") @DateTimeFormat(pattern = "yyyy-MM-dd") LocalDate startDate,
@RequestParam("endDate") @DateTimeFormat(pattern = "yyyy-MM-dd") LocalDate endDate
) {
List<VoucherDto> vouchers = voucherService.findAllBetweenDates(startDate, endDate);

return Response.success(vouchers);
}
Copy link
Author

@legowww legowww Jul 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

조회를 위한 GET Method 사용 시 쿼리스트링이 아닌 메세지 바디에 데이터를 담아서 전송하기도 하나요?
(이 메서드의 경우 메세지 바디에 json 타입의 startDate, endDate 데이터 추가)

검색해보니 14년 이후부터는 GET Method 에서 메세지 바디에 값을 실는 것이 문제없다고 하는데 HTTP 프로토콜과는 잘 맞지 않는 방식 같아서 사용하기 꺼려지네요

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

네 request parameter가 크지 않다면 보통 쿼리파라미터로 받습니다.
바디로 보내실려는 이유가 있을까요?

Copy link
Author

@legowww legowww Jul 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아니오 없습니다!! 단순 질문이었습니다.

@legowww legowww changed the title [4기 - 박형진] SpringBoot Part2 Weekly Mission 두 번째 PR 제출합니다. [4기 - 박형진] SpringBoot Part3 Weekly Mission 두 번째 PR 제출합니다. Jul 17, 2023
Copy link

@hongbin-dev hongbin-dev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

수고하셨습니다 형진님~!


@PostMapping(consumes = MediaType.APPLICATION_JSON_VALUE)
@ResponseStatus(HttpStatus.CREATED)
public Response<Void> create(@RequestBody VoucherCreateRequest voucherCreateRequest) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ResponseEntity를 사용해보면 어떨까요?

https://tecoble.techcourse.co.kr/post/2021-05-10-response-entity/

public Response<Void> create(@RequestBody VoucherCreateRequest voucherCreateRequest) {
voucherService.insert(voucherCreateRequest);

return Response.success(null);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

객체를 생성했을때 200 상태와 null을 리턴하는 방법으로 선택하신 이유가 있나요?
다른 선택지도 있을까요?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ResponseStatus(HttpStatus.CREATED) 때문에 201 상태 아닌가요 ❓

null을 리턴한 이유는 insert 메서드의 반환타입이 void 인 상태여서 마땅히 반환할 값이 없어서 null 을 사용했는데,
ID 값을 반환하도록 변경해보겠습니다.

Comment on lines 37 to 41
List<VoucherDto> vouchers = policy
.map(voucherService::findAllVouchersByPolicy)
.orElse(voucherService.findAllVouchers())
.stream()
.toList();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

해당 비즈니스는 컨트롤러가 아니라 서비스에서 처리해주면 좋겠군요~!

Comment on lines 46 to 54
@GetMapping("/date")
public Response<List<VoucherDto>> betweenDatesCreatedVouchers(
@RequestParam("startDate") @DateTimeFormat(pattern = "yyyy-MM-dd") LocalDate startDate,
@RequestParam("endDate") @DateTimeFormat(pattern = "yyyy-MM-dd") LocalDate endDate
) {
List<VoucherDto> vouchers = voucherService.findAllBetweenDates(startDate, endDate);

return Response.success(vouchers);
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

네 request parameter가 크지 않다면 보통 쿼리파라미터로 받습니다.
바디로 보내실려는 이유가 있을까요?

Comment on lines 46 to 50
@GetMapping("/date")
public Response<List<VoucherDto>> betweenDatesCreatedVouchers(
@RequestParam("startDate") @DateTimeFormat(pattern = "yyyy-MM-dd") LocalDate startDate,
@RequestParam("endDate") @DateTimeFormat(pattern = "yyyy-MM-dd") LocalDate endDate
) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

요기 API는 /api/v1/vouchers와 동일하게 사용할 수 있을 거 같은데요.
나중에 동적쿼리를 적용해봐도 좋을 거 같아요~!

Copy link
Author

@legowww legowww Jul 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

그렇네요
우선 개인과제 마무리해보고 해보겠습니다 감사합니다~

@legowww
Copy link
Author

legowww commented Jul 20, 2023

피드백 반영하여 커밋하였습니다~

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants